Fix(router-core): normalize pipe characters in paths to prevent redirect loops #6293
+119
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughAdds handling and tests for pipe characters in dynamic routes: encodes '|' to '%7C' in path normalization and adds /pipe/$reference routes and e2e tests verifying navigation and encoding. Changes
Sequence Diagram(s)(omitted — changes are focused on path normalization, generated routes, and tests; no multi-component new control flow requiring a diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
@jong-kyung thanks for this. as part of my review process, I like to recreate the error with the e2e tests before validating the fix. I copied the e2e test over into main and the tests passed without a problem. I also tested this manually in the browser and could not replicate the problem. We need to ensure the e2e test fails with the expected error and the solution resolves it correctly to ensure future breakages are avoided. I do see the error in the linked issue but unfortunately, I don't see the source. Would it be possible to link to a github repo or stackblitz link where the error re-occurs? |
Contributor
|
I see now you aren't the author of the original issue. Do you possibly have a recreation of the original problem? |
Contributor
Author
|
Hi @nlynzaad, thanks for reviewing Apologies for the confusion regarding the E2E test. The test passed because I added it to a CSR-only. This issue (ERR_TOO_MANY_REDIRECTS) is specific to SSR environments (TanStack Start), caused by a mismatch between browser encoding and server-side normalization during direct navigation. You can reproduce the issue using the links below:
Could you please confirm the issue using the reproduction links? Once confirmed, please let me know, and I will rewrite the tests to properly target the SSR environment based on your feedback. Thanks again for your time and review, and sorry for the confusion |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 17m 50s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 1m 42s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-01-10 06:03:20 UTC
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @e2e/react-start/basic/src/routes/__root.tsx:
- Around line 184-192: Navbar link for the dynamic pipe route (<Link
to="/pipe/$reference" params={{ reference: 'hello|world' }}>) lacks the trailing
{' '} used by other links and causes spacing drift; add a trailing {' '}
immediately after the closing </Link> for this "Dynamic Pipe Character" link so
it matches the spacing pattern used elsewhere in the nav.
🧹 Nitpick comments (3)
e2e/react-start/basic/src/routes/pipe.$reference.tsx (1)
1-10: LGTM: simple param route that exercises|in a path segment.
One small readability tweak: destructurereferencefromRoute.useParams()since it’s the only field used.Proposed diff
function RouteComponent() { - const params = Route.useParams() - return <h4>Hello {params.reference}!</h4> + const { reference } = Route.useParams() + return <h4>Hello {reference}!</h4> }e2e/react-start/basic/tests/pipe-character-in-path.spec.ts (2)
5-9: Test name doesn’t match behavior (it’s not validating a Linkhref).
If the intent is “param link encodes|”, click the “Dynamic Pipe Character” link and assert the URL (and/or itshref) contains%7C.Proposed diff
-test('encodes pipe character in href for param link', async ({ page }) => { - await page.goto('/pipe/hello|world') - - await expect(page.locator('body')).toContainText('Hello hello|world!') -}) +test('encodes pipe character in href for param link', async ({ page }) => { + await page.goto('/') + await page.getByRole('link', { name: 'Dynamic Pipe Character' }).click() + + await expect(page.locator('body')).toContainText('Hello hello|world!') + expect(new URL(page.url()).pathname).toBe('/pipe/hello%7Cworld') +})
11-24: Make URL assertions less brittle + cover the already-encoded direct navigation case.
baseURLconcatenation can be flaky; asserting onpathnameis usually enough here. Also considergoto('/pipe/hello%7Cworld')to mirror real “reload” behavior.Proposed diff
test('direct navigation keeps encoded url after reload', async ({ page, - baseURL, }) => { await page.goto('/pipe/hello|world') await expect(page.locator('body')).toContainText('Hello hello|world!') - expect(page.url()).toBe(`${baseURL}/pipe/hello%7Cworld`) + expect(new URL(page.url()).pathname).toBe('/pipe/hello%7Cworld') await page.reload() await expect(page.locator('body')).toContainText('Hello hello|world!') - expect(page.url()).toBe(`${baseURL}/pipe/hello%7Cworld`) + expect(new URL(page.url()).pathname).toBe('/pipe/hello%7Cworld') }) + +test('direct navigation to already-encoded url renders without redirects', async ({ page }) => { + await page.goto('/pipe/hello%7Cworld') + await expect(page.locator('body')).toContainText('Hello hello|world!') + expect(new URL(page.url()).pathname).toBe('/pipe/hello%7Cworld') +})
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
e2e/react-start/basic/src/routeTree.gen.tse2e/react-start/basic/src/routes/__root.tsxe2e/react-start/basic/src/routes/pipe.$reference.tsxe2e/react-start/basic/tests/pipe-character-in-path.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
e2e/react-start/basic/src/routes/pipe.$reference.tsxe2e/react-start/basic/tests/pipe-character-in-path.spec.tse2e/react-start/basic/src/routes/__root.tsxe2e/react-start/basic/src/routeTree.gen.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
e2e/react-start/basic/src/routes/pipe.$reference.tsxe2e/react-start/basic/tests/pipe-character-in-path.spec.tse2e/react-start/basic/src/routes/__root.tsxe2e/react-start/basic/src/routeTree.gen.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:35.231Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
e2e/react-start/basic/src/routes/pipe.$reference.tsxe2e/react-start/basic/src/routeTree.gen.ts
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
e2e/react-start/basic/tests/pipe-character-in-path.spec.tse2e/react-start/basic/src/routeTree.gen.ts
📚 Learning: 2025-10-09T12:59:02.129Z
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/src/styles/app.css:19-21
Timestamp: 2025-10-09T12:59:02.129Z
Learning: In e2e test directories (paths containing `e2e/`), accessibility concerns like outline suppression patterns are less critical since the code is for testing purposes, not production use.
Applied to files:
e2e/react-start/basic/tests/pipe-character-in-path.spec.ts
📚 Learning: 2025-10-09T12:59:14.842Z
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/public/site.webmanifest:2-3
Timestamp: 2025-10-09T12:59:14.842Z
Learning: In e2e test fixtures (files under e2e directories), empty or placeholder values in configuration files like site.webmanifest are acceptable and should not be flagged unless the test specifically validates those fields.
Applied to files:
e2e/react-start/basic/tests/pipe-character-in-path.spec.ts
📚 Learning: 2025-12-25T13:04:55.492Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 6215
File: e2e/react-start/custom-basepath/package.json:13-17
Timestamp: 2025-12-25T13:04:55.492Z
Learning: In the TanStack Router repository, e2e test scripts are specifically designed to run in CI (which uses a Unix environment), so Unix-specific commands (like `rm -rf`, `&` for backgrounding, and direct environment variable assignments without `cross-env`) are acceptable in e2e test npm scripts.
Applied to files:
e2e/react-start/basic/tests/pipe-character-in-path.spec.ts
📚 Learning: 2025-10-14T18:59:33.990Z
Learnt from: FatahChan
Repo: TanStack/router PR: 5475
File: e2e/react-start/basic-prerendering/src/routes/redirect/$target/via-beforeLoad.tsx:8-0
Timestamp: 2025-10-14T18:59:33.990Z
Learning: In TanStack Router e2e test files, when a route parameter is validated at the route level (e.g., using zod in validateSearch or param validation), switch statements on that parameter do not require a default case, as the validation ensures only expected values will reach the switch.
Applied to files:
e2e/react-start/basic/tests/pipe-character-in-path.spec.ts
📚 Learning: 2025-12-17T02:17:55.086Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6120
File: packages/router-generator/src/generator.ts:654-657
Timestamp: 2025-12-17T02:17:55.086Z
Learning: In `packages/router-generator/src/generator.ts`, pathless_layout routes must receive a `path` property when they have a `cleanedPath`, even though they are non-path routes. This is necessary because child routes inherit the path from their parent, and without this property, child routes would not have the correct full path at runtime.
Applied to files:
e2e/react-start/basic/src/routeTree.gen.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router
Applied to files:
e2e/react-start/basic/src/routeTree.gen.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Use file-based routing in `src/routes/` directories or code-based routing with route definitions
Applied to files:
e2e/react-start/basic/src/routeTree.gen.ts
📚 Learning: 2025-12-21T12:52:35.231Z
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:35.231Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.
Applied to files:
e2e/react-start/basic/src/routeTree.gen.ts
📚 Learning: 2025-09-28T21:41:45.233Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5284
File: e2e/react-start/basic/server.js:50-0
Timestamp: 2025-09-28T21:41:45.233Z
Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
Applied to files:
e2e/react-start/basic/src/routeTree.gen.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (1)
e2e/react-start/basic/src/routeTree.gen.ts (1)
45-45: Autogenerated route tree update (no manual review).
Per repo guidance, this file should only change via the generator; the added PipeReferenceRoute wiring looks consistent with that expectation.Also applies to: 232-236, 352-353, 399-400, 452-453, 506-507, 553-554, 605-606, 657-658, 898-904, 1207-1208
jong-kyung
force-pushed
the
fix/pipe-char-redirect-loop
branch
from
d29c724 to
4eaf78d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
e2e/react-router/basic-file-based/src/routes/__root.tsx (1)
159-167: Nav link correctly exercises pipe-in-param path viaparams(not manual encoding)This is a solid way to cover the feature in the demo UI without relying on string-concatenated URLs.
Optional: add a test id for easier targeting in future e2e tests
<Link to="/pipe/$reference" params={{ reference: 'hello|world' }} + data-testid="link-to-dynamic-pipe-character" activeProps={{ className: 'font-bold', }} > Dynamic Pipe Character </Link>{' '}e2e/react-router/basic-file-based/tests/app.spec.ts (1)
398-409: Avoid a potentially flaky immediatepage.url()assertion; prefertoHaveURLThis will wait for any normalization redirect/navigation to settle before asserting.
Proposed change
test.describe('Special characters in route paths', () => { test('should render route with pipe character in path', async ({ page, baseURL, }) => { await page.goto('/pipe/hello|world') await expect(page.locator('body')).toContainText('Hello hello|world!') - expect(page.url()).toBe(`${baseURL}/pipe/hello%7Cworld`) + await expect(page).toHaveURL(`${baseURL}/pipe/hello%7Cworld`) }) })Also, since the reported redirect loop is SSR-specific (per #6288 / PR discussion), please verify this test fails on the broken revision and consider adding an SSR-targeted e2e for the TanStack Start reproduction.
e2e/react-router/basic-file-based/src/routes/pipe.$reference.tsx (1)
1-10: Simple, focused route for validating special-character paramsOptional readability tweak
function RouteComponent() { - const params = Route.useParams() - return <h4>Hello {params.reference}!</h4> + const { reference } = Route.useParams() + return <h4>Hello {reference}!</h4> }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
e2e/react-router/basic-file-based/src/routeTree.gen.tse2e/react-router/basic-file-based/src/routes/__root.tsxe2e/react-router/basic-file-based/src/routes/pipe.$reference.tsxe2e/react-router/basic-file-based/tests/app.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
e2e/react-router/basic-file-based/src/routes/pipe.$reference.tsxe2e/react-router/basic-file-based/src/routes/__root.tsxe2e/react-router/basic-file-based/src/routeTree.gen.tse2e/react-router/basic-file-based/tests/app.spec.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
e2e/react-router/basic-file-based/src/routes/pipe.$reference.tsxe2e/react-router/basic-file-based/src/routes/__root.tsxe2e/react-router/basic-file-based/src/routeTree.gen.tse2e/react-router/basic-file-based/tests/app.spec.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:35.231Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
e2e/react-router/basic-file-based/src/routes/pipe.$reference.tsxe2e/react-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
e2e/react-router/basic-file-based/src/routes/pipe.$reference.tsxe2e/react-router/basic-file-based/src/routeTree.gen.tse2e/react-router/basic-file-based/tests/app.spec.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Use file-based routing in `src/routes/` directories or code-based routing with route definitions
Applied to files:
e2e/react-router/basic-file-based/src/routes/pipe.$reference.tsx
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router
Applied to files:
e2e/react-router/basic-file-based/src/routes/pipe.$reference.tsxe2e/react-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-12-17T02:17:55.086Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6120
File: packages/router-generator/src/generator.ts:654-657
Timestamp: 2025-12-17T02:17:55.086Z
Learning: In `packages/router-generator/src/generator.ts`, pathless_layout routes must receive a `path` property when they have a `cleanedPath`, even though they are non-path routes. This is necessary because child routes inherit the path from their parent, and without this property, child routes would not have the correct full path at runtime.
Applied to files:
e2e/react-router/basic-file-based/src/routes/pipe.$reference.tsxe2e/react-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-12-21T12:52:35.231Z
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:35.231Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.
Applied to files:
e2e/react-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-10-09T12:59:02.129Z
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/src/styles/app.css:19-21
Timestamp: 2025-10-09T12:59:02.129Z
Learning: In e2e test directories (paths containing `e2e/`), accessibility concerns like outline suppression patterns are less critical since the code is for testing purposes, not production use.
Applied to files:
e2e/react-router/basic-file-based/tests/app.spec.ts
📚 Learning: 2025-12-25T13:04:55.492Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 6215
File: e2e/react-start/custom-basepath/package.json:13-17
Timestamp: 2025-12-25T13:04:55.492Z
Learning: In the TanStack Router repository, e2e test scripts are specifically designed to run in CI (which uses a Unix environment), so Unix-specific commands (like `rm -rf`, `&` for backgrounding, and direct environment variable assignments without `cross-env`) are acceptable in e2e test npm scripts.
Applied to files:
e2e/react-router/basic-file-based/tests/app.spec.ts
📚 Learning: 2025-10-14T18:59:33.990Z
Learnt from: FatahChan
Repo: TanStack/router PR: 5475
File: e2e/react-start/basic-prerendering/src/routes/redirect/$target/via-beforeLoad.tsx:8-0
Timestamp: 2025-10-14T18:59:33.990Z
Learning: In TanStack Router e2e test files, when a route parameter is validated at the route level (e.g., using zod in validateSearch or param validation), switch statements on that parameter do not require a default case, as the validation ensures only expected values will reach the switch.
Applied to files:
e2e/react-router/basic-file-based/tests/app.spec.ts
🧬 Code graph analysis (1)
e2e/react-router/basic-file-based/src/routes/pipe.$reference.tsx (1)
e2e/react-router/basic-file-based/src/routes/__root.tsx (1)
Route(12-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
jong-kyung
force-pushed
the
fix/pipe-char-redirect-loop
branch
from
df7f149 to
8a683d9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #6288
This PR fixes an issue where using a pipe character (
|) in route parameters caused an infinite redirect loop (ERR_TOO_MANY_REDIRECTS)The Problem
Browsers automatically encode
|characters to%7Cin the URL when navigating directly. However, the router's internal normalization logic (cleanPath) was treating|as a raw character.This caused a mismatch between the current location (encoded by browser) and the expected location (unencoded by router), triggering a redirect loop where the router tried to redirect to the unencoded path, but the browser re-encoded it upon request.
The Fix
I updated the
cleanPathutility function to explicitly normalize|characters to%7C. This ensures that the router's internal path representation matches the browser's encoding behavior, preventing the unnecessary redirect loop.Changes
cleanPathto replace|with%7C.Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.